refactor(breadbox): Service layer proposal #93
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Following up from discussion here and in person:
@pgm and @jessica-cheng it seemed like you were both enthusiastic about creating a "service layer" in breadbox so that we have a more typical 3-tiered setup. I personally am very excited about this because I'm finding the existing setup increasingly confusing and I think a service layer would be helpful for the DE2 support I am currently working on. I wanted to ask what that would look like in your minds, but I thought it might be easier to discuss a tangible example.
I am not attached to anything here, so feel free to disagree if you see something you don't like (for example, I'm not sure my naming is the best), and we can talk more at the breadbox meeting tomorrow if that's easier than giving feedback here.
Currently, breadbox has two "tiers":
api
(endpoint definitions)crud
(contains almost all of our functions that do any data reading/writing, not just low-level crud operations)I would propose our 3 tiers look something like:
api
- only contains endpoint definitions (this is already the case)service
- has higher-level logic about how the app should function. Nothing here should include SQLAlchemy queries.crud
- reusable functions which are directly querying the database (all SQLAlchemy queries would still go here)We could slowly move service-type-logic out of the crud layer to get things into a more organized state. I did some low-hanging fruit here and set up a service module with 3 files, each containing a few functions:
labels.py
:get_dataset_feature_labels_by_id
get_dataset_sample_labels_by_id
get_dataset_feature_by_label
get_dataset_sample_by_label
slice.py
:get_slice_data
get_labels_for_slice_type
data_loading.py
:get_subsetted_matrix_dataset_df